feat: Add `session$destroy()` to remove all module reactivity by schloerke · Pull Request #4372 · rstudio/shiny

schloerke · 2026-04-17T16:50:15Z

Summary

Port of posit-dev/py-shiny#2209 — adds session$destroy() and session$onDestroy() to R Shiny.

Adds session$destroy() on module session proxies to destroy all reactive state (values, expressions, observers, inputs, outputs) for a module scope and its descendants
Adds session$onDestroy(callback) to register cleanup callbacks that fire on destroy
Reactive primitives (ReactiveVal, Observable, Observer, ReactiveValues) auto-register via weak references during init, so destruction is automatic and comprehensive
ReactiveValues gains a full destroy() method with domain auto-registration, destroyed guards on get()/set(), and a destroyByPrefix() method for namespace-scoped cleanup
invalidateLater() timers are cancelled on both session end and module destroy, with cross-deregistration to prevent leaks
Observer onDestroy registration respects the autoDestroy flag
Destroy callbacks fire deepest-namespace-first (children before parents)
Accessing a destroyed reactive raises a loud shiny.destroyed.error
createMockDomain() supports onDestroy/destroy lifecycle (destroy fires on end())
Reserved namespace ..root is rejected in makeScope()
registerBookmarkExclude() returns an unsubscribe handle, cleaned up on module destroy
No OTel/rLog emissions during destruction

Closes #2281 — The canonical issue: insertUI()/callModule() to add, removeUI()/??? to remove. No mechanism to deactivate a module server instance.

Closes #825 — "Reactive subDomains" proposal (2016). Proposes createSubDomain() where ending the subdomain destroys all reactive objects created within it.

Related to #2374 — Request to delete server-side input values on removeUI(). Fix is to wrap the UI component into a module. Then removal of all reactivity (not just the input) becomes: mod_session$destroy().

What this means for app authors

When a module's UI is actually removed from the DOM — via removeUI(), or by re-rendering a uiOutput/renderUI so the old UI is replaced — only the UI goes away. The module's server-side reactivity — its observers, reactive values, and reactive expressions — keeps running invisibly in the background. In a long-running app that adds and removes modules repeatedly (tabs, list items, editor panes), this "dangling reactivity" accumulates: observers keep firing, invalidateLater() timers keep ticking, and memory grows over the life of the session.

This PR gives you a way to clean all of that up. The parent owns the child's lifecycle: it created the module under an id, it removes that module's UI, and it tears down the module's reactivity under the same id — no need to thread a cleanup handle back out of the module:

# Parent server: create the module
myModuleServer("editor")

# ...later, when the module's UI is removed:
removeUI(selector = "#editor")   # or however the UI leaves the DOM
session$destroy("editor")        # tear down the module's reactivity

session$destroy(id) tears down every reactive object created in that module's scope (and any nested child modules), and runs any cleanup you registered with session$onDestroy(). Crucially, it's scoped: the parent session and sibling modules are untouched, so it's safe to destroy one module without affecting the rest of the app.

One rule to keep in mind: data that must outlive a module should be created outside it. A reactiveVal() created inside a module is destroyed with the module, so reading it afterward errors. If you need a result to survive, create the reactive value in the caller and pass it into the module — it lives in the caller's scope and remains valid after the module is gone.

Key design decisions

Weak references via rlang::new_weakref() allow reactive objects to be GC'd before explicit destroy() — matching py-shiny semantics
ShinySession$destroy() throws an error — only module session proxies can be destroyed; root session uses close()
Destroy callbacks stored centrally on root session in a Map keyed by namespace string
make_weak_destroy_wrapper() helper avoids closure environment leaks (discovered via GC tests)
ReactiveValues domain auto-registration — user-created reactiveValues() inside a reactive domain auto-register for destroy (like reactiveVal); session-level stores (input/clientData) use destroyByPrefix() for namespace-scoped cleanup instead
Cross-deregistration pattern for invalidateLater() — onEnded and onDestroy callbacks each deregister the other when they fire, preventing double-cleanup and leaked handles

Files changed

File	Changes
`R/reactives.R`	`destroyedReactiveError`, `ReactiveVal$destroy()`, `Observable$destroy()`, Observer weak registration with `autoDestroy` gating, `ReactiveValues$destroy()` + `destroyByPrefix()` + domain auto-registration, `make_weak_destroy_wrapper()`
`R/shiny.R`	`ShinySession` destroy infrastructure, `makeScope()` proxy overrides, `wsClosed()` integration, `registerBookmarkExclude()` unsubscribe handle, `..root` namespace guard
`R/reactive-domains.R`	`createMockDomain()` with `onDestroy`/`destroy` support
`R/mock-session.R`	`MockShinySession` destroy support, bookmark-exclude real implementations, `..root` namespace guard
`R/insert-ui.R`	`@section` on `removeUI` for destroy usage
`R/modules.R`	`@section` on `moduleServer` for destroy usage, realistic docs example
`tests/testthat/test-destroy.R`	99 tests covering all destroy scenarios including weakref GC, invalidateLater timers, bookmark-exclude cleanup, ReactiveValues destroy
`tests/testthat/test-mock-session.R`	Updated bookmark-exclude tests for real implementations
`tests/testthat/test-test-server.R`	Fixed test using reactiveValues as state tracker after session close

🤖 Generated with Claude Code

Port of posit-dev/py-shiny#2209 — adds session$destroy() and session$onDestroy() to clean up dangling reactivity when dynamic module UI is removed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add tests that verify weakref GC semantics work correctly with destroy callbacks for ReactiveVal, Observable, and Observer. Also fix two bugs discovered by the tests: 1. Inline closures in initialize() captured the entire enclosing environment (including `self`/`private`), preventing GC. Fixed by extracting to make_weak_destroy_wrapper() helper. 2. R's lazy evaluation meant the `wr` argument to the helper was a promise retaining a reference to initialize()'s execution env (which holds `self`). Fixed by adding force(wr). 3. Storing self$destroy as the weakref value created a strong reference cycle. Fixed by using wref_key() instead of wref_value() and calling obj$destroy() on the retrieved key. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Removes all keys matching a namespace prefix from the reactive values store, invalidates their dependents, and notifies names/list watchers. This enables cleanup of module inputs when a module scope is destroyed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…nySession Adds destroyCallbacksByNs (Map of namespace -> Callbacks) to both ShinySession and MockShinySession, with public onDestroy()/destroy() methods and private helpers for namespace-scoped callback invocation and resource cleanup (inputs, outputs, downloads, files). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…ssion docs for onDestroy/destroy Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Adds an explicit module-scope teardown API to Shiny sessions to eliminate “dangling reactivity” when dynamic module UI is removed, with reactive primitives participating automatically via weakly-registered destroy hooks.

Changes:

Introduces session$destroy() / session$onDestroy() for module session proxies, backed by namespace-keyed destroy callback infrastructure on the root session.
Adds destroy semantics/guards to core reactive primitives (ReactiveVal, Observable, Observer) plus ReactiveValues$_destroy() for namespace cleanup.
Adds comprehensive destroy-focused test coverage and documentation/NEWS updates.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`R/shiny.R`	Implements destroy callback registry on `ShinySession`, adds proxy `onDestroy()`/`destroy()`, and invokes destroy on websocket close.
`R/reactives.R`	Adds destroyed error condition + destroy methods and weak onDestroy registration for reactive primitives; adds `ReactiveValues$_destroy()`.
`R/mock-session.R`	Adds destroy callback infra and proxy destroy support for `MockShinySession` to enable testing.
`tests/testthat/test-destroy.R`	New unit tests covering destroyed guards, idempotency, weakref behavior, and module-scope teardown ordering.
`man/session.Rd`	Documents `session$onDestroy()` and `session$destroy()`.
`man/insertUI.Rd`	Documents how `session$destroy()` relates to `removeUI()` for module cleanup.
`R/insert-ui.R`	Adds roxygen section describing module cleanup with `session$destroy()`.
`man/MockShinySession.Rd`	Documents new `MockShinySession$onDestroy()` / `$destroy()`.
`NEWS.md`	Announces new session destroy APIs and behavior.
`.gitignore`	Ignores `docs/` and `.context`.
`.Rbuildignore`	Excludes `docs` and `.context` from package builds.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Use ns.sep constant instead of hardcoded "-" in destroy logic - Sort root sentinel __root__ last during full-session teardown - Use rlang::names2() for NULL-safe output name lookup - Clean up output-related clientData entries (output_<ns>-* pattern) - MockShinySession: delegate close() to invokeDestroyCallbacks for proper deepest-first ordering and input cleanup - Always clean up inputs/outputs/routes even when no destroy callbacks are registered (remove early return before cleanup) - Rename misleading weakref test description - Add tests for close ordering, input cleanup, and root-last behavior Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…I docs - Add "Module lifecycle and composability" section to ?session with data ownership examples (return-from-module pitfall vs pass-in pattern) - Add "Destroying module reactivity" section to ?moduleServer - Update ?removeUI to cross-reference ?session composability docs - Clarify destroy() invokes onDestroy callbacks, with cleanup as consequence Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

schloerke · 2026-05-29T15:58:02Z

The parent already knows the id
Yes, but I don't want to use the id twice if I can help it

"modules return reactives"
Really, that's modules return data and handlers... this is a handler

feels most natural to me is the parent owns the child's lifecycle

session$destroyScope(id) would be session$makeScope(id)$destroy(). Similar to other reactive objects, each object owns its own destroy method. Moving the method to the parent session would break this pattern.

Also, most people don't know about makeScope(id). But will do have access to the current module's session. If they need to perform the destroy elsewhere, then they can use makeScope as a workaround.

cpsievert · 2026-05-29T16:14:44Z

Do you view it as an anti-pattern to destroy a module from the parent session? My initial reaction is that this would be the more common pattern, which is why I think a more ergonomic API would make sense.

Similar to other reactive objects, each object owns its own destroy method. Moving the method to the parent session would break this pattern.

I see what you mean, but it also feels a little bit odd to me to motivate a harder-to-discover API based on this. The fact that we have a method for making scope also argues to me that it's sensible to one for the reverse operation. That said, maybe it is a little bit weird considering that $makeScope() isn't really public?

cpsievert · 2026-05-29T16:44:37Z

After chatting, we think session$destroy() should be able to take an id and have it become session$makeScope(id)$destroy()

mlechon · 2026-05-29T16:59:56Z

Hi! I'm really looking forward to this feature. Thanks for taking the time to implement it!

Have you considered triggering destroy automatically on a module's session when a call to moduleServer(id = "previously_used_id") happens? I can't think of a situation in which one would want the two servers to coexist, so collecting the first one implicitly could be a nice touch.

This may not make sense if you're trying to keep the R and Python APIs aligned. And I can always write a wrapper that does this, so I'll be happy anyways :)

schloerke · 2026-05-29T17:12:16Z

Have you considered triggering destroy automatically on a module's session when a call to moduleServer(id = "previously_used_id") happens? I can't think of a situation in which one would want the two servers to coexist, so collecting the first one implicitly could be a nice touch.

Oooooo, thank you @mlechon ! I like the intent, but it feels too magical.

I do agree that this is a nice place to raise an error and to tell the app author to call parent_session$destroy(id)

I'll go ahead and have it raise and error if any previous reactive objects exist when calling moduleServer("existing_id")

schloerke · 2026-05-29T17:18:17Z

Am going to move the request to an Issue to separate it from this PR (and to give it it's own number)

@cpsievert

session$destroy() now accepts an optional `id`. When supplied, it is equivalent to session$makeScope(id)$destroy(), letting a parent tear down a module using the same id it used to insert the UI — no handle threading required. This works on the root session too, where session$destroy() with no id remains an error (root is closed via close()). The no-arg session$destroy() on a module proxy is unchanged: each scope still owns its own destroy method. Adds a validateDestroyId() guard and updates the ?session, ?moduleServer, and ?removeUI docs to lead with the parent-driven pattern, keeping the "return a handle" pattern as the secondary option. Resolves the API discussion with @cpsievert on #4372. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Drop "equivalent to session$makeScope(id)$destroy()" from NEWS and the ?session/?moduleServer/?removeUI prose; lead users to session$destroy(id) rather than teaching the makeScope() backdoor - Update the destroy() method item to document the optional id and add a removeUI() + destroy(id) example - Split the session "Module lifecycle and composability" section into "Destroying a module" and "Module data ownership" Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

khusmann · 2026-05-29T18:41:15Z

Thank you all for your continued work on this!

The mental model that feels most natural to me is the parent owns the child's lifecycle: it inserted the UI under an id, it removes the UI, it destroys the server under the same id. That'd argue for a first-class, symmetric entry point

I 100% agree with this -- I think the parent owning the child lifecycle is the most natural mental model. It's also the most general, I think: a child owning its own lifecycle only works in a subset of special cases.

In fact, I might even go as far as to say it's an antipattern for the child to self-destroy / touch its own lifecycle, because in general you can leave the parent with dangling references. (But I'd need to stew / think deeper on this to say this confidently).

So yeah, I think the session$destroy(id) you've converged on for this is the right move here!

I'll go ahead and have it raise and error if any previous reactive objects exist when calling moduleServer("existing_id")

I think this is good. In the same spirit can we also error (or at least warn) if we session$destroy(id) on a module that doesn't exist? (This will also help catch typos and stale ids)

schloerke · 2026-05-29T19:10:05Z

I'll go ahead and have it raise and error if any previous reactive objects exist when calling moduleServer("existing_id")

I think this is good. In the same spirit can we also error (or at least warn) if we session$destroy(id) on a module that doesn't exist? (This will also help catch typos and stale ids)

Warnings for both would make sense. An error for currently "working" code is jarring.

Need to see how duplicate output values are handled for a notification cadence.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Sweep the pre-existing \code{} usages in defineOutput() and showProgress() inline docstrings to backticks for consistency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@khusmann

session$destroy(id) now emits a warning (class "shiny.destroy.unknown_id") when the id names a scope that was never created or has already been destroyed, helping catch typos and stale ids. It's a warning rather than an error so it doesn't break otherwise-working code, matching how Shiny treats duplicate output ids (always warn). Adds a private scopeExists() check to ShinySession and MockShinySession that runs before makeScope(id) (which would otherwise register its own callbacks and mask the check). Addresses @khusmann's request on #4372. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

schloerke · 2026-05-29T19:34:07Z

Adding no warnings for both.

Currently, the behavior for calling $destroy(id) means that all known reactivity under that ID is removed. Calling it twice still holds true, even if the second call is a no-op.

Removed the warning.

callModule(server, character(0)) reaches makeScope(character(0)). #4372's new destroy-callback code branched on if (!nzchar(namespace)) to detect the root scope, but nzchar(character(0)) is logical(0), so the guard errored with "argument is of length zero". A length-0 namespace is documented NS() behavior meaning "no namespace" (root) and worked before #4372 (makeScope had no nzchar guard). Treat a length-0 namespace as root only in the destroy-key logic, leaving the namespace itself untouched so NS(character(0)) keeps returning ids unchanged (normalizing to "" would instead prefix ids with a stray separator).

Default the public destroy()/onDestroy() entry points and the internal invokeDestroyCallbacks() to NULL instead of character(0), and lead with NULL in the docstrings and guard messages. NULL is the idiomatic R 'absence' value and was the original #4372 public default. character(0) remains accepted (both are length 0, which is how root is detected everywhere), so callers like teal that pass character(0) are unaffected; '' and NA are still rejected.

schloerke and others added 18 commits April 16, 2026 16:11

docs: add session$destroy() design spec

5213c4b

Port of posit-dev/py-shiny#2209 — adds session$destroy() and session$onDestroy() to clean up dangling reactivity when dynamic module UI is removed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

chore: add docs/ to .gitignore and .Rbuildignore

c73e0bd

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

feat: add destroyedReactiveError condition constructor

7cd36fc

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

feat: add destroy() method and guards to ReactiveVal

f295282

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

feat: add destroy() method and guards to Observable

ca4d1f9

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

feat: Observer auto-registers weak destroy callback with domain

1dc8b36

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

feat: ReactiveVal and Observable auto-register weak destroy callbacks

faa8a65

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

feat: add onDestroy/destroy to session proxies via makeScope

92391a4

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

test: add integration tests for full module destroy flow

5b24569

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

docs: add roxygen for onDestroy/destroy, @Seealso on removeUI

9cce3b9

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

docs: add NEWS entry for session$destroy()

584c7ba

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

devtools::document() (GitHub Actions)

9ca13cc

docs: update NEWS PR number placeholder to #4372

1ab5519

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

docs: replace broken ShinySession @Seealso with usage snippet, add se…

e1a8e09

…ssion docs for onDestroy/destroy Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

chore: add .context to .gitignore and .Rbuildignore

b2c9c38

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

schloerke requested a review from Copilot April 17, 2026 20:42

schloerke commented Apr 17, 2026

View reviewed changes

Comment thread R/mock-session.R Outdated

schloerke commented Apr 17, 2026

View reviewed changes

Comment thread R/shiny.R Outdated

schloerke commented Apr 17, 2026

View reviewed changes

Comment thread R/shiny.R

schloerke commented Apr 17, 2026

View reviewed changes

Comment thread R/shiny.R Outdated

Copilot AI reviewed Apr 17, 2026

View reviewed changes

Comment thread R/shiny.R Outdated

Comment thread R/shiny.R Outdated

Comment thread R/mock-session.R Outdated

Comment thread tests/testthat/test-destroy.R Outdated

schloerke and others added 4 commits April 20, 2026 11:26

docs: polish NEWS entry to match tidyverse style and updated docs

7603141

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

fix: improve destroy() error messages with actionable guidance

4a13407

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

schloerke requested a review from Copilot April 20, 2026 16:23

Copilot started reviewing on behalf of schloerke April 20, 2026 16:24 View session

schloerke mentioned this pull request May 29, 2026

feat: Error when calling moduleServer(id="existing") #4392

Closed

schloerke and others added 2 commits May 29, 2026 13:58

schloerke and others added 2 commits May 29, 2026 15:14

docs: use backticks instead of \code{} in R6 destroy method docstrings

33c1088

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

docs: use backticks instead of \code{} in remaining R6 method docstrings

1cf4d10

Sweep the pre-existing \code{} usages in defineOutput() and showProgress() inline docstrings to backticks for consistency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

schloerke force-pushed the schloerke/py-shiny-2209-port branch from 1d1da60 to 1cf4d10 Compare May 29, 2026 19:32

schloerke merged commit cceb4e4 into main May 29, 2026
22 of 24 checks passed

schloerke deleted the schloerke/py-shiny-2209-port branch May 29, 2026 19:40

schloerke mentioned this pull request May 29, 2026

feat: Add id argument to session.destroy() posit-dev/py-shiny#2264

Merged

cpsievert mentioned this pull request Jun 1, 2026

fix: support running a module in the parent namespace via an empty id #4394

Closed

cpsievert mentioned this pull request Jun 1, 2026

fix: support NULL as the canonical root namespace for session$destroy(), etc #4395

Draft

This was referenced Jun 3, 2026

Detach generate_plugin_args() output from its testServer session BristolMyersSquibb/blockr.core#198

Merged

Replace internal-reaching module cleanup with shiny's session$destroy() BristolMyersSquibb/blockr.core#202

Open

Conversation

schloerke commented Apr 17, 2026 • edited by cpsievert Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What this means for app authors

Key design decisions

Files changed

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

schloerke commented May 29, 2026

Uh oh!

cpsievert commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cpsievert commented May 29, 2026

Uh oh!

mlechon commented May 29, 2026

Uh oh!

schloerke commented May 29, 2026

Uh oh!

schloerke commented May 29, 2026

Uh oh!

khusmann commented May 29, 2026

Uh oh!

schloerke commented May 29, 2026

Uh oh!

schloerke commented May 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

schloerke commented Apr 17, 2026 •

edited by cpsievert

Loading

cpsievert commented May 29, 2026 •

edited

Loading